-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Build IDL Anchor 0.30 #48
Conversation
Co-authored-by: Christian <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the TODO here? Check the validator list address to the config?
programs/steward/src/constants.rs
Outdated
@@ -10,7 +10,7 @@ pub const EPOCH_PROGRESS_MAX: f64 = 0.99; | |||
// Cannot go more than 100 epochs without scoring | |||
pub const NUM_EPOCHS_BETWEEN_SCORING_MAX: u64 = 100; | |||
// Cannot score validators in under 100 slots, to submit 1 instruction per validator | |||
pub const COMPUTE_SCORE_SLOT_RANGE_MIN: usize = 100; | |||
pub const COMPUTE_SCORE_SLOT_RANGE_MIN: u64 = 100; | |||
#[cfg(feature = "mainnet-beta")] | |||
pub const VALIDATOR_HISTORY_FIRST_RELIABLE_EPOCH: usize = 520; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just change these right away to u64?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe address the TODO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe make BITMASK_SIZE and all other const variables u64
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For variables that are explicitly used for indexing into arrays or defining size I think we should keep them as usize, as long as they don't need to be exposed in the IDL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe make the index a uXX as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make all constants uXX
@CoachChuckFF I have a task in the "additional tweaks to steward" list to add validator_list to config, skipping for now since it may move around some data in the account so waiting until a future deploy of the program |
@@ -388,9 +425,10 @@ impl StewardState { | |||
.num_pool_validators | |||
.checked_sub(1) | |||
.ok_or(StewardError::ArithmeticError)?; | |||
let num_pool_validators = self.num_pool_validators as usize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks a lot nicer!
@@ -10,7 +10,7 @@ use spl_stake_pool::{ | |||
|
|||
use crate::{errors::StewardError, Config, Delegation}; | |||
|
|||
pub fn get_stake_pool(account: &AccountLoader<Config>) -> Result<Pubkey> { | |||
pub fn get_stake_pool_address(account: &AccountLoader<Config>) -> Result<Pubkey> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good to me!
… 1.18. remove after solana updates
anchor-lang = "0.30.0" | ||
anchor-lang = { features = ["idl-build"], version = "0.30.0" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I was just passing by and saw you've enabled the idl-build
feature for the default target. This should be avoided, as the IDL generation process is a separate build process that doesn't use Solana's build tools.
This seems harmless now other than adding a bit of unnecessary compilation, but it could very well break the compilation of your program in the future. For this reason, I added warnings for this usage in coral-xyz/anchor#3061.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@acheroncrypto thanks for the input, will make the change!
Recommendation from #48. Should help compilation issues downstream.
Fixes to build IDL for new Anchor 0.30 spec. New IDL spec breaking changes:
usize
not supported in instructions or accounts